-
Notifications
You must be signed in to change notification settings - Fork 54
Light Calorimetry #878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Light Calorimetry #878
Conversation
Version v09_67_00
- cleaned (but need to clean more) skipping products that aren't used (xarapuca flashes + truth products) for example to avoid nullptr
- cleaned (but need to clean more) skipping products that aren't used (xarapuca flashes + truth products) for example to avoid nullptr
- moved truth validation to a separate function - added lower/upper threshold for PE per channel - added fcl param to turn on/off evaluating all planes - option to add dir/refl efficiencies; NOTE: not really accounted for in the code yet
- merge the two previous trees into one - match-type can now differentiate between event/slice level easily
Version v09_89_01, patch release for SBN2024A
Version v09_93_01_02p02
Version v10_04_07
asanchezcastillo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together @lynnt20, this is a lot of great work! I just left some minor comments, mainly about fcl inheritance which I think is strongly recommended in this case.
…to some fcl comments - also remove unused functions
|
Thanks for all the comments and suggestions @asanchezcastillo !! I think I've addressed everything, let me know if you see anything else! |
|
Adding sim/reco group conveners for additional sign-off.
We will merge with any of the conveners sign-off on it from the physics & organization perspective. Lynn's CM presentation here: |
linyan-w
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! One nitpicking comment: Is the Noise Threshold and Max Threshold unique to light calorimetry? If so great. Otherwise it would be nice to check that's the globally used values.
@linyan-w They are unique to this module, so no globally used values here. |
henrylay97
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general this looks great @lynnt20! Thanks for the comprehensive approach. Also you should get an award for perseverance in not letting this branch die!
A few small comments, some of which can be ignored at your discretion.
…ing messages; avoid hardcoded TPC tick
This reverts commit 6ff86f1.
…e data fcl configuration
Version v10_14_02_01
|
Can you resolve merge conflicts when you get a chance? Thanks! |
|
@sungbinoh, I added functionality to access TPC Calibration database to grab the electron lifetime for data. Would you mind reviewing that part of the code (producer module and data flc)? Thank you! |
henrylay97
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the swift updates, looks great!
|
|
||
| // Query database - translate run into fake "timestamp" | ||
| // (same convention as NormalizeDriftSQLite) | ||
| felifetime_db->UpdateData((run + 1000000000) * 1000000000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comment for all of us, and this can be punted to a separate PR. Should we have a single place where the fake "timestamp" is formed? So that when we inevitably end up changing the prescription in future we only have one point of maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sungbinoh may have thoughts
Description
Add producer module to create light calorimetry data products. Creates new subdirectory
Calorimetryto store the module and fcls. Over 3 years since the first commit! Reco2/caf level changes only.Checklist
Reviewers,AssigneesDevelopementRelevant PR links
Accompanying PRs: SBNSoftware/sbncode#619, SBNSoftware/sbnanaobj#181, SBNSoftware/sbnobj#158
Link(s) to docdb describing changes (optional)
Is there a docdb describing the issue this solves or the feature added?